Skip to content

EDM-4157: [SPUR] Add divider before Delete#706

Open
bugs-buddy-jira-ai-issue-solver[bot] wants to merge 4 commits into
flightctl:mainfrom
celdrake:bugs-buddy-jira-ai-issue-solver/EDM-4157
Open

EDM-4157: [SPUR] Add divider before Delete#706
bugs-buddy-jira-ai-issue-solver[bot] wants to merge 4 commits into
flightctl:mainfrom
celdrake:bugs-buddy-jira-ai-issue-solver/EDM-4157

Conversation

@bugs-buddy-jira-ai-issue-solver

@bugs-buddy-jira-ai-issue-solver bugs-buddy-jira-ai-issue-solver Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Resolves EDM-4157

Summary

Add PatternFly dividers before destructive actions in all action dropdown menus across the application.

Fixes EDM-4157

Details

Destructive actions (Delete, Decommission, Remove, Cancel) were listed in dropdown menus without visual separation from non-destructive actions, violating PatternFly conventions and increasing risk of accidental data loss.

This PR adds standard PatternFly dividers before the first destructive action in every action menu:

  • Table row kebab menus (7 files): Uses { isSeparator: true } in IAction[] arrays, which PatternFly's ActionsColumn natively renders as a <Divider />
  • Detail page action dropdowns (6 files): Uses <Divider component="li" /> inside <DropdownList>
  • Catalog page action dropdown (1 file): Uses <Divider component="li" />

Dividers are conditionally rendered to avoid showing when the destructive action is the only menu item.

Affected menus

Page Destructive action
Enrolled devices table Decommission device
Decommissioned devices table Delete device
Device details Decommission device
Fleets table Delete fleet
Fleet details Delete fleet
Image builds table Cancel / Delete image build
Image build details Cancel / Delete image build
Repositories table Delete repository
Repository details Delete repository
Auth providers table Delete
Auth provider details Delete authentication provider
Enrollment requests table Delete
Enrollment request details Delete
Catalog page Remove

Test plan

  • Verify TypeScript compilation passes (tsc --noEmit)
  • Verify ESLint passes on all modified files
  • Verify Prettier formatting passes on all modified files
  • Open each table listing page and verify a divider appears before destructive actions in kebab menus
  • Open each detail page and verify a divider appears before destructive actions in the Actions dropdown
  • Verify no divider appears when the destructive action is the only menu item (e.g., user with only delete permission)
  • Verify catalog page shows divider before "Remove" action

@bugs-buddy-jira-ai-issue-solver @celdrake
EDM-4157: [SPUR] Add divider before Delete
6280108 
Co-authored-by: Celia Amador Gonzalez <camadorg@redhat.com>
@celdrake celdrake force-pushed the bugs-buddy-jira-ai-issue-solver/EDM-4157 branch from dfae9a3 to 057963b Compare June 17, 2026 10:52
jkilzi
jkilzi approved these changes Jun 21, 2026
View reviewed changes

@jkilzi jkilzi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, comprehensive refactor. The two-tier API is well-designed: the JSX compound component (ActionsDropdownList + Item) handles DropdownList usages, while the buildAllDropdownActions helper handles IAction[] arrays for ActionsColumn table rows. Both produce a consistent separator between regular and danger groups.

The mechanical nature of the change (same pattern applied to ~20 files) reduces per-file risk, and CI passing validates correctness.

Good — ImageExportCards action ordering. Moving delete to the end of the actions list (danger group) is the right UX change.

Note — child.type reference equality. ActionsDropdownList uses child.type === ActionsDropdownListItem to classify children. This is a valid React pattern but can theoretically fail in extreme HMR or module-isolation scenarios. Low production risk, but a brief JSDoc comment on the component explaining the classification convention would help future maintainers.

Nit — bundled label rename. "Remove" → "Delete catalog" in CatalogPage.tsx is a welcome clarity improvement, but it's bundled silently into a PR titled "Add divider before Delete". Worth calling out in the description so translators don't miss it.

Nit — ActionsDropdownListItem renders as a fragment. The isDanger prop is intentionally unused by the component itself (the parent reads it via child.props.isDanger). This is a valid React convention but unusual enough to warrant a comment for maintainers.

@bugs-buddy-jira-ai-issue-solver @celdrake
EDM-4157: address PR feedback
b6bb5ca 
Co-authored-by: Celia Amador Gonzalez <camadorg@redhat.com>
@bugs-buddy-jira-ai-issue-solver

bugs-buddy-jira-ai-issue-solver Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

In comment, @jkilzi said:

Clean, comprehensive refactor. The two-tier API is well-designed: the JSX compound component (ActionsDropdownList + Item) handles DropdownList usages, while the buildAllDropdownActions helper handles IAction[] arrays for ActionsColumn table rows. Both produce a consistent separator between regular and danger groups.

The mechanical nature of the change (same pattern applied to ~20 files) reduces per-file risk, and CI passing validates correctness.

Good — ImageExportCards action ordering. Moving delete to the end of the actions list (danger group) is the right UX change.

Note — child.type reference equality. ActionsDropdownList uses child.type === ActionsDropdownListItem to classify children. This is a valid React pattern but can theoretically fail in extreme HMR or module-isolation scenarios. Low production risk, but a brief JSDoc comment on the component explaining the classification convention would help future maintainers.

Nit — bundled label rename. "Remove" → "Delete catalog" in CatalogPage.tsx is a welcome clarity improvement, but it's bundled silently into a PR titled "Add divider before Delete". Worth calling out in the description so translators don't miss it.

Nit — ActionsDropdownListItem renders as a fragment. The isDanger prop is intentionally unused by the component itself (the parent reads it via child.props.isDanger). This is a valid React convention but unusual enough to warrant a comment for maintainers.

Added JSDoc comments addressing both notes: (1) ActionsDropdownList now documents the child.type reference-equality classification convention for future maintainers, (2) ActionsDropdownListItem now explains that isDanger is intentionally read by the parent, not the component itself. The bundled label rename ("Remove" → "Delete catalog") is a PR description concern — acknowledged, but no code change needed.

Addressed in b6bb5ca.

@celdrake

celdrake commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Needs re-approval after last commit adding JSDoc comments.

@celdrake celdrake requested a review from jkilzi June 29, 2026 09:41
@bugs-buddy-jira-ai-issue-solver

bugs-buddy-jira-ai-issue-solver Bot commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

AI Session Costs

Session Cost
Feedback #1 $1.16
Feedback #2 $0.81
Feedback #3 $0.80
Feedback #4 $1.04
Feedback #5 $0.52
Feedback #6 $0.38
Feedback #7 $0.89
Feedback #8 $0.60
Feedback #9 $0.71
Total $6.91

@bugs-buddy-jira-ai-issue-solver

bugs-buddy-jira-ai-issue-solver Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

In comment, @celdrake said:

Needs re-approval after last commit adding JSDoc comments.

I was unable to produce code changes to address this comment after multiple attempts.

@celdrake

celdrake commented Jun 29, 2026 •
edited by amir-yogev-gh
Loading

Copy link
Copy Markdown
Collaborator

@bugs-buddy-jira-ai-issue-solver Ignore the following comment
This is a message for a human :)

@bugs-buddy-jira-ai-issue-solver

bugs-buddy-jira-ai-issue-solver Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

In comment, @celdrake said:

@@bugs-buddy-jira-ai-issue-solver Ignore the following comment
This is a message for a human :)

I was unable to produce code changes to address this comment after multiple attempts.

@amir-yogev-gh

amir-yogev-gh commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

@bugs-buddy-jira-ai-issue-solver Ignore the following comment
This is a message for a human :)

@bugs-buddy-jira-ai-issue-solver

bugs-buddy-jira-ai-issue-solver Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

In comment, @amir-yogev-gh said:

@bugs-buddy-jira-ai-issue-solver Ignore the following comment
This is a message for a human :)

I was unable to produce code changes to address this comment after multiple attempts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkilzi jkilzi Awaiting requested review from jkilzi

At least 1 approving review is required to merge this pull request.

Assignees

@celdrake celdrake

Labels

ai-pr release-1.3 size=L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@celdrake @amir-yogev-gh @jkilzi